-
Notifications
You must be signed in to change notification settings - Fork 15
Add KaTeX rendering engine #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Thank you for this @ongchi
|
|
The main changes needed
|
|
Hi @zerolab, I should find some time to update the README and add some relevant tests in this PR. |
aa9f67f to
4be578f
Compare
zerolab
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay. Did a first pass review. Need to check the widget side of things and test it locally in another pass.
Left some minor suggestions/notes
| @@ -0,0 +1,58 @@ | |||
| from django.conf import settings | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note to self: move settings to a lazy object a la Django settings
src/wagtail_polymath/config.py
Outdated
| paths = WAGTAILPOLYMATH_SETTINGS.get(key, []) | ||
|
|
||
| # Return absolute path to the asset if it's a static file path. | ||
| return [versioned_static(path) if finders.find(path) else path for path in paths] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we skip running finders.find if path starts with http ?
src/wagtail_polymath/config.py
Outdated
| paths = POLYMATH_SETTINGS.get(key, []) | ||
|
|
||
| # Return absolute path to the asset if it's a static file path. | ||
| return [versioned_static(path) if finders.find(path) else path for path in paths] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be good to have this URL validation, so I am adding validation logic for a generic URL.
|
Hello there! We just wanted to include math support in one of our project, and we find wagtailkatex :) Seeing this PR may leed us to use wagtail-polymath instead. Is this PR still going? We might spend some time to upgrade it to support wagtail 7 also if needed. |
|
Sorry for the late reply! I’m planning to split this PR into a few smaller ones to make it easier to review. I’ve manually tested it, and everything seems to work fine from Wagtail 5.2 up to 6.1. However, the latest version (7.1) hasn’t been tested yet. |
This PR include some breaking changes:
WAGTAIL_POLYMATH: The default value of rendering engine ismathjax.katexis also supported. Can also specify specific version of rendering engine from CDN, or from self provisioned django static files.wagtailmathtowagtail_polymath